setup.sh: install prepare-commit-msg hook for Crow-Session trailer#519
Merged
Conversation
) The auto-merge gate requires a `Crow-Session: <uuid>` trailer matching a known session on at least one PR commit. `setup.sh` writes `.claude/settings.local.json` with `attribution.commit` so Claude Code's built-in commit flow lands the trailer — but hand-rolled `git commit -m` / heredoc commits skip that flow and produce trailerless commits. radiusmethod/corveil#1442 was the live proof: session 4CF06A61-… was active, settings.local.json was correct, and commit 0780a100 still shipped with no `Crow-Session:` line, so the gate refused auto-merge. This change closes the bypass with a per-worktree `prepare-commit-msg` hook that idempotently appends `Crow-Session:` and `Co-Authored-By:` when missing, plus matching worker-prompt guidance so the hook is a safety net rather than the only line of defense. Worktree-scoped via `extensions.worktreeConfig` + per-worktree `core.hooksPath` so it never pollutes sibling worktrees of the same repo. The hook body lives in one canonical heredoc copied verbatim into setup.sh and the bundled template; AttributionSkillTests guards against drift. Opt-out path (`attributionTrailers: false`) also affirmatively removes a pre-existing install. Tests: - skills/crow-workspace/setup_hook_test.sh — 27 cases covering append, idempotence, foreign-trailer preservation, empty body, comment-only body, merge/squash sources, missing session id, worktree isolation (commit in sibling worktree carries no trailer), real `git commit` in the configured worktree carries both trailers, opt-out removes hook + CROW_SESSION_ID. - Tests/CrowTests/AttributionSkillTests.swift — 4 new snapshots: byte-identical install_commit_hook between live and template, live SKILL.md teaches trailer requirement, template SKILL.md teaches trailer requirement, FOOTER carries the Committed row. - Manual #1442 repro: installed the hook in the corveil-1441 sibling worktree under the same 4CF06A61-… session id; `git commit --allow-empty -m "…"` produced a commit carrying both trailers, a second `--amend` left the message byte-identical, and the crow-518 worktree itself stayed hook-free (isolation across repos). Reverted the smoke commit (HEAD back at 0780a100) and uninstalled the hook from corveil-1441 before pushing. 🐦⬛ Generated with Claude Code, orchestrated by Crow Co-Authored-By: Claude <noreply@anthropic.com> Crow-Session: DC5F9D9D-4F14-4711-97B2-733C8F489804
dhilgaertner
approved these changes
Jun 16, 2026
dhilgaertner
left a comment
Contributor
There was a problem hiding this comment.
Code & Security Review
Closes #518 by installing a per-worktree prepare-commit-msg hook that idempotently appends the Crow-Session: <uuid> and Co-Authored-By: Claude trailers, closing the bypass where hand-rolled git commit -m/heredoc commits skipped Claude Code's attribution.commit flow and defeated the crow:merge gate.
I verified the core mechanics directly rather than relying on the PR description:
- Hook output matches the gate. The hook emits
Crow-Session: <uuid>viagit interpret-trailers, which produces a blank-line-separated, line-anchored trailer block that parses cleanly againstIssueTracker.crowSessionTrailerPattern(^Crow-Session:\s*([0-9A-Fa-f-]{36})\s*$with.anchorsMatchLines). Confirmed. - Per-worktree scoping is sound. I confirmed empirically that
git rev-parse --git-dirreturns an absolute path for linked worktrees (which Crow always creates viagit worktree add), so thecore.hooksPathandCROW_SESSION_IDresolution holds. The--git-dir-not---git-path hookschoice correctly avoids polluting the shared$GIT_COMMON_DIR/hooks. - Shell harness passes locally:
setup_hook_test.sh→ 27 passed, 0 failed (install, idempotency, foreign-UUID preservation, merge/squash no-op, empty-session no-op, sibling-worktree isolation, opt-out cleanup). - Drift guard is real: the added lines in
setup.shandcrow-workspace-setup.sh.templateare byte-identical, andworkspaceSetupAndTemplateHookBlocksAreByteIdenticalenforces it going forward.
Security Review
Strengths:
- Hook never blocks a commit:
set -u, noset -e,exit 0on every path, allgitcalls|| true/2>/dev/null. A malformed environment degrades to a trailerless commit, not a brokengit commit. - Session id is read from a gitdir-local
CROW_SESSION_IDfile and only flows intogit interpret-trailers --trailer, not intoevalor a shell-interpolated context — no injection surface. - Empty/comment-only message detection preserves git's "abort on empty message" behavior instead of silently materializing a commit by growing a body — a thoughtful safety choice.
- Foreign user-typed
Crow-Session:trailers are preserved (grep guard skips re-adding), and existingCo-Authored-By: Claudeis not duplicated. - Opt-out (
attributionTrailers: false) removes both the hook file andCROW_SESSION_ID, so on→off flips clean up.
Concerns: none.
Code Quality
- POSIX
#!/bin/sh, portable utilities (grep -E,tr,interpret-trailers); guards ongitavailability and gitdir resolution. - Worker-prompt guidance (SKILL.md, FOOTER.md,
CrowAttributionconstant) mirrored across live + bundled + Swift-constant surfaces, each backed by a test.
Considerations (non-blocking)
- Interactive-editor commits are intentionally out of scope.
prepare-commit-msgruns before the editor opens, so for a baregit committhe body is comment-only at hook time and the hook no-ops — a freshly-typed editor message won't receive trailers. This is the correct tradeoff (agents usegit commit -m, the stated target; covering it would risk un-aborting empty commits), but worth noting as a documented boundary. extensions.worktreeConfig truecarries git's standard one-time caveat that sharedcore.worktree/core.bareshould be migrated to the main worktree's config. Negligible for normally-cloned repos with linked worktrees, but noting it for completeness.- The "absolute path" comment on
core.hooksPathis precisely true only for linked worktrees (always the case in Crow); for a main worktree--git-diris relative, though git still resolves it correctly from the working-tree root.
Summary Table
| Color | Meaning | Verdict effect |
|---|---|---|
| Red | Must fix | Request changes |
| Yellow | Should fix | Request changes |
| Green | Consider | Approve allowed |
Recommendation: Approve — driven by [0 Red, 0 Yellow, 3 Green] findings. The fix is correct, well-scoped, isolated per-worktree, fail-open, and covered by both a passing shell harness and Swift drift guards.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #518.
The
crow:mergeauto-merge gate (docs/automation.md, IssueTracker.crowSessionTrailerPattern) requires at least one commit on the PR to carry aCrow-Session: <uuid>trailer matching a known session.setup.shwrites.claude/settings.local.jsonwithattribution.commitso Claude Code's built-in commit flow lands the trailer — but hand-rolledgit commit -m "…"/ heredoc commits bypass it and produce trailerless commits.RadiusMethod/corveil#1442 is the live proof: session
4CF06A61-…was active,settings.local.jsonwas correct, and commit0780a100shipped with noCrow-Session:line, so the gate refused auto-merge.This PR closes the bypass.
What changes
skills/crow-workspace/setup.shandResources/crow-workspace-setup.sh.template(mirrored, byte-identical) gaininstall_commit_hook+remove_commit_hook.main()callsinstall_commit_hookafterwrite_settings_local. The function:is_attribution_trailers_enabledhelper — same opt-out (attributionTrailers: false) assettings.local.json.extensions.worktreeConfigon the main repo (idempotent).core.hooksPathto this worktree'sgitdir/hooks(computed offgit rev-parse --git-dir, NOT--git-path hooks— the latter falls back to$GIT_COMMON_DIR/hookswhich would pollute every sibling worktree).CROW_SESSION_IDfile under the per-worktree gitdir (resolved viagit rev-parse --git-path CROW_SESSION_ID).prepare-commit-msghook into that hooks dir.Opt-out (
remove_commit_hook) deletes both the hook and theCROW_SESSION_IDfile so a flip from on→off cleans up the install.The hook script itself is a single canonical heredoc. It:
$2 == merge/squash(merge/squash messages get crafted server-side).grep -vE '^[[:space:]]*#').git rev-parse --git-path CROW_SESSION_ID. Missing/empty → no-op.Crow-Session:when anyCrow-Session:line already exists — preserves a user-typed trailer even with a different UUID.Co-Authored-By: Claudewhen present.git interpret-trailers --in-placecall so the resulting block is blank-line-separated, line-anchored, and parses cleanly against^Crow-Session:\s*([0-9A-Fa-f-]{36})\s*$with.anchorsMatchLines.set -u, no-e,exit 0on every path).Worker-prompt guidance mirrored across live + bundled template:
skills/crow-workspace/SKILL.mdandResources/crow-workspace-SKILL.md.template— the existing### Commit Attribution Trailerssection now explicitly tells the worker to include both trailers when authoring commits by hand and calls out the hook as the safety net.skills/crow-attribution/FOOTER.md,Resources/crow-attribution-FOOTER.md.template, andCrowAttribution.sharedFooterInstructions(Swift constant) gain a "Committed" row matching the existing Created / Reviewed rows.Per-worktree scoping
Default
gitshares.git/hooks/across all worktrees of a repo, so a naïve install would pollute every sibling worktree (different worktrees may belong to different sessions, or none). The combined fix:git config --local extensions.worktreeConfig true.git/config(shared)git config --worktree core.hooksPath <abs>config.worktree.git/worktrees/<name>/hooks/prepare-commit-msgCROW_SESSION_IDfile.git/worktrees/<name>/CROW_SESSION_IDWorktree-isolation is verified by Tests G + G2 in the shell harness AND by the manual #1442 repro below (the crow-518 worktree itself stayed hook-free while corveil-1441 received and used the hook).
Tests
Shell harness —
skills/crow-workspace/setup_hook_test.sh, 27 cases mapped to the ticket's acceptance criteria:install_commit_hookfrom a worktreecore.hooksPath,extensions.worktreeConfigall land in the right placesCrow-Session:line, exactly 1Co-Authored-By:line\$2 == merge/\$2 == squashCROW_SESSION_IDfileCROW_SESSION_IDgit commitin wt-aattributionTrailers: false, theninstall_commit_hookruns againCROW_SESSION_IDare removedbash skills/crow-workspace/setup_hook_test.sh→ 27 passed, 0 failed. Gateway tests unchanged (17 passed).Swift snapshots — 4 new tests in
Tests/CrowTests/AttributionSkillTests.swift:workspaceSetupAndTemplateHookBlocksAreByteIdentical— guards theinstall_commit_hook/remove_commit_hookblock byte-for-byte between live setup.sh and the bundled template; partial copy = silent loss of fix in new scaffolds.liveWorkspaceSkillTeachesTrailerRequirement— live SKILL.md mentions both trailer strings and the hook name.bundledWorkspaceTemplateTeachesTrailerRequirement— same against the bundled template.attributionFooterContainsCommittedRow— FOOTER carries the new Committed row.arch -arm64 swift test --arch arm64→ 230 passed across 25 suites. (ExistingliveAttributionFooterAndBundledTemplateAreByteIdenticalandliveAttributionFooterMatchesSwiftConstantcontinue to pass — all three FOOTER surfaces gained the same Committed row.)Manual #1442 repro
In the sibling worktree at
/Users/danny/Projects/devroot/rm/corveil-1441-root-redirect-ui— the very target of #1442:Sourced this PR's
install_commit_hook, ran it withSESSION_ID=4CF06A61-C504-4A95-8C44-8C8246B0F703, the exact id called out in the ticket.Verified install: hook at
.git/worktrees/corveil-1441-root-redirect-ui/hooks/prepare-commit-msg,CROW_SESSION_IDfile alongside,core.hooksPathset per-worktree,extensions.worktreeConfig=trueon the main corveil repo.git commit --allow-empty -m "CROW-518 smoke: …"→ resulting log message:git commit --amend --allow-empty --no-edit→ message stays byte-identical (1Crow-Session:line, 1Co-Authored-By:line).Confirmed the crow-518 worktree itself (sibling worktree of a DIFFERENT repo) had no hook installed → isolation works across repos.
Reverted:
git reset --hard HEAD~1(HEAD back at0780a100),rm -fthe hook + CROW_SESSION_ID. The corveil-1441 worktree is left exactly as we found it.Test plan
bash skills/crow-workspace/setup_hook_test.sh— 27/27 pass.bash skills/crow-workspace/setup_gateway_test.sh— 17/17 pass (no regressions).arch -arm64 swift test --arch arm64— 230 tests across 25 suites pass.🐦⬛ Generated with Claude Code, orchestrated by Crow